Fixes partial path resolvings for jbuilder partials. #220
Conversation
Fixes partial path resolvings for jbuilder partials
…tial_rendering Update themes.rb
|
Hey @pgruener, this looks like a small enough change, but I don't really understand why we need it. What would be some reproduction steps to see the error that you're trying to prevent? (I see the comment in the code you changed, but I don't quite get it. Is there a particular combination of gems that was giving you trouble?) |
Hi @jagthedrummer unfortunately there is so much time past, that I actually don't remember which gem caused the error. My fault that I wasn't precise enough. And my opinion was, that as long as this is not a forbidden pattern, it should not crash the framework. So actually you should be able to reproduce it, by making this call by yourself in an own ressource template. |
For me it was a hard journey to discover this error, especially reasoned by the condition if Rails.env.development?, which lets the error only occur in production environment. And I was looking for some trouble (TypeError: no implicit conversion of nil into Integer) in the affected jbuilder template, which was totally wrong.
I am sorry, that I can't afford the time to write tests for that and I understand, if that's why the PR won't be merged. But as I investigated several hours in finding this issue, I (at least) wanted to point this error out.
So if you didn't ever plan that this method is called with a path with another class-instance than String, you could easily approve it. Otherwise I hope this PR helps somebody as reference, as we implemented that as a monkey patch for now.
Let me know if I should clarify the situation more that I reached with my comment.